fix: Use raw string values instead of repr() for str types in display_string by kgilpin · Pull Request #383 · getappmap/appmap-python

kgilpin · 2026-03-31T23:48:04Z

Replace isinstance(val, str) with issubclass(type(val), str) to avoid __class__ lookups on proxy/lazy objects
Update the display_string docstring/comment to reflect that str types return raw values (not repr())
Verify all tests pass

…_string When APPMAP_DISPLAY_PARAMS=true, repr() wraps string values in quotes (e.g. 'my-api-key'), which breaks the scanner's secret-in-log substring matching. By using the raw string value directly for str types, the recorded value matches what appears in log messages, enabling proper secret leak detection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adjusts AppMap parameter/return-value rendering so str values are recorded as their raw string content (not repr()-quoted), improving consistency with log output and enabling reliable secret-in-log substring matching.

Changes:

Update display_string to return raw str values instead of repr(val) for strings.
Update unit tests to assert raw string values for parameters/return values.
Update JSON “expected” AppMap fixtures to match the new string formatting.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`_appmap/event.py`	Changes `display_string` to emit raw `str` values instead of `repr()` output.
`_appmap/test/web_framework.py`	Updates web framework capture assertions to expect raw string values.
`_appmap/test/test_params.py`	Updates parameter-capture assertions/parameterized expectations for `str` values.
`_appmap/test/test_http.py`	Updates HTTP client capture assertions for raw string query param values.
`_appmap/test/test_events.py`	Updates labeled params/return value assertions and comment to reflect raw strings.
`_appmap/test/data/unittest/expected/unittest.appmap.json`	Updates expected recorded parameter/return `str` values (and formatting) to raw strings.
`_appmap/test/data/unittest/expected/unittest-no-test-cases.appmap.json`	Updates expected recorded `str` values to raw strings.
`_appmap/test/data/unittest/expected/pytest.appmap.json`	Updates expected recorded `str` values to raw strings.
`_appmap/test/data/pytest/expected/pytest-numpy2.appmap.json`	Updates expected recorded `str` return values to raw strings.
`_appmap/test/data/pytest/expected/pytest-numpy2-no-test-cases.appmap.json`	Updates expected recorded `str` return values to raw strings.
`_appmap/test/data/pytest/expected/pytest-numpy1.appmap.json`	Updates expected recorded `str` return values to raw strings.
`_appmap/test/data/pytest/expected/pytest-numpy1-no-test-cases.appmap.json`	Updates expected recorded `str` return values to raw strings.
`_appmap/test/data/expected.appmap.json`	Updates expected recorded `str` parameter/return values (including newline escaping) to raw strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T23:52:01Z

_appmap/event.py

    if _should_display(has_labels):
        try:
-            value = repr(val)
+            value = val if isinstance(val, str) else repr(val)


Using isinstance(val, str) here can trigger class lookups on proxy/lazy objects (the file already avoids isinstance for this reason in _is_list_or_dict), potentially causing side effects or premature evaluation when params display is enabled. Consider avoiding isinstance and instead check the real type (e.g., type(val) is str or issubclass(type(val), str)) before returning the raw value.

Suggested change

value = val if isinstance(val, str) else repr(val)

val_type = type(val)

value = val if (val_type is str or issubclass(val_type, str)) else repr(val)

What about:

issubclass(type(val), str)

Changed to issubclass(type(val), str) in commit 622ec72.

Copilot · 2026-03-31T23:52:02Z

_appmap/event.py

    if _should_display(has_labels):
        try:
-            value = repr(val)
+            value = val if isinstance(val, str) else repr(val)


The comment above still states that parameter display uses repr() to compute a string value, but the implementation now returns the raw value for str types. Please update the comment to reflect the new behavior so it doesn't mislead future readers.

@copilot apply changes based on this feedback

Updated the comment in commit 622ec72 to clarify that str types are returned as-is and other types use repr().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Agent-Logs-Url: https://github.com/getappmap/appmap-python/sessions/164d7ad2-ff95-401b-a976-40cbc36ab6a5 Co-authored-by: kgilpin <86395+kgilpin@users.noreply.github.com>

dividedmind · 2026-04-01T21:24:37Z

CLAUDE.md

+appmap-python pytest
+
+# Also works for quick local iteration on non-subprocess tests
+APPMAP=true .venv/bin/python -m pytest _appmap/test/test_events.py


Note many tests require APPMAP_DISPLAY_PARAMS=true (which tox sets across the board).

Also note uv run is an alternative to using venv directly.

dividedmind

Looks good to me. We went back and forth on whether strings should be represented in maps verbatim or as a representation; the scanner usage does finally provide a good argument on one over the other.

The test is also a good addition.

Before merging, please:

fixup the issubclass and comment fix into the main feature commit,
amend the main feature commit message to include BREAKING CHANGE: note; this changes the format of the stringified parameters.

One thing I wonder: since we're special casing strings, does it make sense to always capture them, regardless of appmap_display_params value? If it's a string, there is should be no risk of extra serialization overhead. Two potential pitfalls: 1) making sure the class is also always captured in this case, 2) do we want to truncate to avoid putting huge strings in the appmap?

kgilpin requested review from Copilot and dividedmind March 31, 2026 23:49

kgilpin self-assigned this Mar 31, 2026

Copilot started reviewing on behalf of kgilpin March 31, 2026 23:49 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

docs: Add CLAUDE.md with test running instructions

01592d8

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot started work on behalf of kgilpin April 1, 2026 00:02 View session

fix: use issubclass(type(val), str) and update display_string comment

622ec72

Agent-Logs-Url: https://github.com/getappmap/appmap-python/sessions/164d7ad2-ff95-401b-a976-40cbc36ab6a5 Co-authored-by: kgilpin <86395+kgilpin@users.noreply.github.com>

Copilot finished work on behalf of kgilpin April 1, 2026 00:04

chore: Test that labeled functions are always recorded

eb77b1c

dividedmind reviewed Apr 1, 2026

View reviewed changes

dividedmind approved these changes Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Use raw string values instead of repr() for str types in display_string#383

fix: Use raw string values instead of repr() for str types in display_string#383
kgilpin wants to merge 4 commits intofeat/display-labeled-paramsfrom
fix/strip-repr-quotes-from-strings

kgilpin commented Mar 31, 2026 •

edited by Copilot AI

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

kgilpin Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

kgilpin Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

dividedmind Apr 1, 2026

Uh oh!

dividedmind left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	value = val if isinstance(val, str) else repr(val)
	val_type = type(val)
	value = val if (val_type is str or issubclass(val_type, str)) else repr(val)

Conversation

kgilpin commented Mar 31, 2026 • edited by Copilot AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

kgilpin Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

kgilpin Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

dividedmind Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

dividedmind left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kgilpin commented Mar 31, 2026 •

edited by Copilot AI

Loading

dividedmind left a comment •

edited

Loading